GH-49268: [C++][FlightRPC] Fix ODBC tests for MacOS by justing-bq · Pull Request #49267 · apache/arrow

justing-bq · 2026-02-12T19:55:24Z

Rationale for this change

Addresses #49268

What changes are included in this PR?

Fixed Mock Server setup to work on both Windows and MacOS.
Changed Mocker Server setup logic to happen once at the Test Environment level.
Changed test connect/disconnect logic to happen once per test fixture instead of once per test.
Disabled tests that are not appliable to MacOS (usually because iODBC doesn't support the relevant functionality).
Adjusted expected SQL States where different on Mac.
Fixed any tests failing on MacOS.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

GitHub Issue: [C++][FlightRPC] Fix ODBC tests for MacOS #49268

github-actions · 2026-02-12T19:55:50Z

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

Pull request overview

This PR aims to make the Arrow Flight SQL ODBC test suite pass on macOS (iODBC) by restructuring test setup/teardown (mock server + connection lifecycle), adjusting platform-specific expectations, and enabling the ODBC tests to run in macOS CI.

Changes:

Reworked ODBC test fixtures to share global ODBC handles and move connection/setup logic to test-suite scope (plus added a global mock-server test environment).
Adjusted/disabled tests and expectations for iODBC/macOS differences (SQL states, unsupported APIs).
Updated DSN/system-DSN plumbing and macOS CI permissions so DSN-based tests can run.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
cpp/src/arrow/flight/sql/odbc/tests/type_info_test.cc	Refactors handle usage and adds macOS-specific behavior for ODBC v2 type-info cases.
cpp/src/arrow/flight/sql/odbc/tests/tables_test.cc	Updates tests to use shared statement handle pattern; adds explicit table cleanup.
cpp/src/arrow/flight/sql/odbc/tests/statement_attr_test.cc	Improves error assertions; enables/updates statement attribute tests.
cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h	Introduces shared global handles/state and macOS buffer sizing; refactors fixture APIs to static helpers.
cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc	Adds global mock server environment; refactors connect/disconnect lifecycle; enables DSN write helpers on more platforms.
cpp/src/arrow/flight/sql/odbc/tests/get_functions_test.cc	Disables SQLGetFunctions tests on macOS/iODBC.
cpp/src/arrow/flight/sql/odbc/tests/errors_test.cc	Updates tests to use shared handles and macOS-specific behavior.
cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc	Updates tests to use shared handles; enables DSN-based connection tests beyond Windows.
cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc	Adjusts expected values for macOS DM; adds handle-only variants for inconsistent DM behavior.
cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc	Adds macOS/iODBC-specific expectations and disables tracing-related cases on macOS.
cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt	Adjusts link order to prefer iODBC on macOS.
cpp/src/arrow/flight/sql/odbc/odbc_impl/win_system_dsn.cc	Updates PostError call signature usage.
cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.h	Moves DSN API into shared impl headers; adjusts PostError signature; adds odbcinst include.
cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.cc	Makes DSN registration helpers build cross-platform; changes PostError signature/behavior.
cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc	Avoids descriptor null deref when setting APD/ARD attrs.
cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt	Builds system_dsn.{cc,h} as part of core ODBC impl library.
cpp/src/arrow/flight/sql/example/sqlite_tables_schema_batch_reader.cc	Improves schema-reading logic by mapping columns per table in a single SQLite scan.
ci/scripts/cpp_test.sh	Stops excluding the Flight SQL ODBC test binary on macOS.
.github/workflows/cpp_extra.yml	Grants macOS CI write permission to system DSN location.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-12T07:09:46Z

cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.cc

@@ -42,7 +38,7 @@ void PostArrowUtilError(arrow::Status error_status) {
  std::wstring werror_msg = arrow::util::UTF8ToWideString(error_msg).ValueOr(
      L"Error during utf8 to wide string conversion");

-  PostError(ODBC_ERROR_GENERAL_ERR, werror_msg.c_str());
+  PostError(ODBC_ERROR_GENERAL_ERR, (LPWSTR)werror_msg.c_str());
 }

 void PostLastInstallerError() {
@@ -55,7 +51,7 @@ void PostLastInstallerError() {
  buf << L"Message: \"" << msg << L"\", Code: " << code;
  std::wstring error_msg = buf.str();

-  PostError(code, error_msg.c_str());
+  PostError(code, (LPWSTR)error_msg.c_str());
 }


PostError/callers cast away constness from std::wstring::c_str() to pass an LPWSTR. If the installer API ever writes into this buffer, that becomes undefined behavior. Prefer keeping the parameter const (e.g., LPCWSTR) when possible, or pass a truly mutable, null-terminated buffer (e.g., std::vector<wchar_t> / std::wstring with data() in C++17+ and ensuring null termination) instead of casting.

Now using const_cast.

Copilot · 2026-03-12T07:09:47Z

cpp/src/arrow/flight/sql/odbc/tests/type_info_test.cc

-TEST_F(TypeInfoOdbcV2MockTest, TestSQLGetTypeInfoSQLTypeTimeODBCVer2) {
+TEST_F(TypeInfoOdbcV2MockTest, TestSQLGetTypeInfoSQLTypeTime) {
+#ifdef __APPLE__
+  ASSERT_EQ(SQL_SUCCESS, SQLGetTypeInfo(stmt, SQL_TYPE_DATE));


On macOS branch of TestSQLGetTypeInfoSQLTypeTime, the test calls SQLGetTypeInfo with SQL_TYPE_DATE instead of SQL_TYPE_TIME, so it isn't actually exercising the time type and will give misleading coverage/results. Update the argument to match the test name/intent.

Suggested change

ASSERT_EQ(SQL_SUCCESS, SQLGetTypeInfo(stmt, SQL_TYPE_DATE));

ASSERT_EQ(SQL_SUCCESS, SQLGetTypeInfo(stmt, SQL_TYPE_TIME));

Copilot · 2026-03-12T07:09:47Z

cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.cc

-#include "arrow/flight/sql/odbc/odbc_impl/util.h"
 #include "arrow/result.h"
 #include "arrow/util/utf8.h"



system_dsn.cc uses boost::iequals in RegisterDsn(...) but the Boost predicate header is no longer included after this change, which will break compilation. Add the appropriate Boost include (e.g., the predicate header) in this file or in a header that guarantees it for all build targets.

Suggested change

#include <boost/algorithm/string/predicate.hpp>

alinaliBQ · 2026-03-12T17:10:25Z

Could you check the "ODBC Windows" CI failure?
https://github.com/apache/arrow/actions/runs/22965712197/job/66668619896?pr=49267

There is an issue for this: #49465. I think arrow-flight-test (Timeout) failure is caught on MSVC Windows platform, and I think it is not likely caused by ODBC-related changes as we haven't modified the Flight code.

kou · 2026-03-13T01:50:12Z

There is an issue for this: #49465.

Oh, sorry. I missed it.

kou

+1

conbench-apache-arrow · 2026-03-13T08:05:36Z

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 21a2d4f.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.

github-actions bot added Component: C++ Component: FlightRPC CI: Extra: C++ Run extra C++ CI awaiting review Awaiting review labels Feb 12, 2026

justing-bq marked this pull request as ready for review February 12, 2026 20:48

justing-bq requested a review from lidavidm as a code owner February 12, 2026 20:48

justing-bq changed the title ~~[C++][FlightRPC] Fix tests for MacOS~~ [C++][FlightRPC] Fix ODBC tests for MacOS Feb 12, 2026

justing-bq changed the title ~~[C++][FlightRPC] Fix ODBC tests for MacOS~~ GH-49268: [C++][FlightRPC] Fix ODBC tests for MacOS Feb 12, 2026

justing-bq marked this pull request as draft February 12, 2026 21:08

alinaliBQ force-pushed the fixMacTests branch 6 times, most recently from 53d0e44 to af8035e Compare March 10, 2026 17:42

alinaliBQ reviewed Mar 11, 2026

View reviewed changes

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Mar 11, 2026

justing-bq marked this pull request as ready for review March 11, 2026 17:22

justing-bq requested review from assignUser, jonkeane, kou and raulcd as code owners March 11, 2026 17:22

alinaliBQ and others added 3 commits March 11, 2026 10:24

Resolve Unix-odbc vs. IODBC linking issue

795a1d0

Put `ODBCINST` before `ODBC_LIBRARIES` in terms of linking. Co-Authored-By: Alina (Xi) Li <alinal@bitquilltech.com>

Change GetInfo disabled tests to start new connection

5c6228c

* Enable disabled tests * Fix getinfo tests to use standalone connection

justing-bq force-pushed the fixMacTests branch from 31f1de8 to 5c6228c Compare March 11, 2026 17:28

kou reviewed Mar 12, 2026

View reviewed changes

kou requested a review from Copilot March 12, 2026 07:01

Copilot started reviewing on behalf of kou March 12, 2026 07:01 View session

github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Mar 12, 2026

Copilot AI reviewed Mar 12, 2026

View reviewed changes

Address comments

dfe4dc9

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Mar 12, 2026

kou approved these changes Mar 13, 2026

View reviewed changes

kou merged commit 21a2d4f into apache:main Mar 13, 2026
79 of 81 checks passed

kou removed the awaiting change review Awaiting change review label Mar 13, 2026

github-actions bot added the awaiting merge Awaiting merge label Mar 13, 2026

kou mentioned this pull request Mar 13, 2026

[C++][FlightRPC] Fix ODBC tests for MacOS #49268

Closed

+                ValidateFetch(stmt, SQL_NO_DATA);
                 // Sizes are zeros
-                EXPECT_EQ(SQL_SUCCESS, SQLColumns(this->stmt, catalog_name, 0, schema_name, 0,

	ASSERT_EQ(SQL_SUCCESS, SQLGetTypeInfo(stmt, SQL_TYPE_DATE));
	ASSERT_EQ(SQL_SUCCESS, SQLGetTypeInfo(stmt, SQL_TYPE_TIME));

Conversation

justing-bq commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions bot commented Feb 12, 2026

Uh oh!

justing-bq commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 12, 2026

Uh oh!

alinaliBQ Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

kou Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

justing-bq Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

justing-bq Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

justing-bq Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

justing-bq Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

alinaliBQ commented Mar 12, 2026

Uh oh!

kou commented Mar 13, 2026

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

conbench-apache-arrow bot commented Mar 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

justing-bq commented Feb 12, 2026 •

edited

Loading

justing-bq commented Feb 12, 2026 •

edited

Loading